feat(featureflags): add build-kernel-version flag for new builds#2592
feat(featureflags): add build-kernel-version flag for new builds#2592ValentaTomas merged 5 commits intomainfrom
Conversation
Mirror the existing build-firecracker-version flag with a build-kernel-version flag so the kernel baked into NEW template builds can be rolled forward without a code change. Existing snapshots/builds keep their pinned kernel version. Flag name matches what orbit (belt) already references.
PR SummaryMedium Risk Overview The API still includes kernel/firecracker versions in build requests while the orchestrator now resolves kernel via feature flags too, so inconsistent flag context or future partial removal on one side could cause mismatched/pinned versions and reduce build reproducibility. Reviewed by Cursor Bugbot for commit 127ca29. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 9 Tests Failed:
View the full list of 10 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
There was a problem hiding this comment.
LGTM — straightforward mirror of the existing build-firecracker-version flag.
Extended reasoning...
Overview
This PR adds a BuildKernelVersion LaunchDarkly string flag that mirrors the existing BuildFirecrackerVersion flag, lets the API handlers and orchestrator resolve the kernel version baked into new template builds via LD instead of from Config.DefaultKernelVersion/featureflags.DefaultKernelVersion, and introduces an opt-in NewStringFlagFallbackOnEmpty constructor so an empty LD string also falls through to the fallback. Five files touched; the diff is mechanical.
Security risks
None identified. No auth, crypto, permission, or input-validation surfaces are touched. The feature flag plumbing is additive and the new fallbackWhenEmpty field is opt-in (only BuildKernelVersion uses it), so behavior of every existing StringFlag is unchanged.
Level of scrutiny
Low–medium. The change influences which kernel image new builds get pinned to, but existing snapshots/builds keep their stored env_builds.kernel_version (set at build time and not re-read on resume), and the LD default is wired through the same DEFAULT_KERNEL_VERSION env var → in-tree DefaultKernelVersion constant chain that the previous code used. The shape is identical to the already-shipped BuildFirecrackerVersion flag.
Other factors
The only bug surfaced is a nit (the Config.DefaultKernelVersion field — already marked // Deprecated — and its post-parse default-fill block are now write-only) and is explicitly out of scope. The integration-tests CI failure shown in the timeline is a 'matrix result: cancelled' aggregate, not a real test failure. Recent commits (refactor(featureflags): bake empty-fallback into StringFlag opt-in, fix(featureflags): fall back to DefaultKernelVersion when LD flag is empty) show the author already iterated on the empty-fallback semantics in response to earlier feedback.
Code Review by Qodo
1. BuildFirecrackerVersion missing empty-string fallback
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Mirrors the existing
build-firecracker-versionflag with a newbuild-kernel-versionflag so the kernel baked into NEW template builds can be rolled forward via LaunchDarkly without a code change. Existing snapshots/builds keep their pinned kernel version (env_builds.kernel_versionis set at build time and not touched on resume).Flag name matches what orbit (belt) already references (
LD_FLAG_BUILD_KERNEL_VERSION = 'build-kernel-version').Default value falls back to
DEFAULT_KERNEL_VERSIONenv var, then to the in-treeDefaultKernelVersionconstant — same shape as the FC flag.